fix: add stream log and fix add node options #394
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds tracer injection for streaming calls: CompiledStateGraphObject.stream now prepends the LangGraphTracer to Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AppController
participant CompiledStateGraph as CompiledStateGraph.stream
participant LangGraphTracer as Tracer
participant ServerLog as Log
Client->>AppController: HTTP GET /llm/graph
AppController->>CompiledStateGraph: call stream/invoke(input, config with tags:['trace-log'])
CompiledStateGraph->>CompiledStateGraph: detect 'trace-log' tag
CompiledStateGraph->>Tracer: obtain/prepend tracer to callbacks
CompiledStateGraph->>CompiledStateGraph: call original stream/invoke with augmented callbacks
CompiledStateGraph-->>AppController: yield chunks / final result
Tracer->>ServerLog: write trace entries (e.g., [agent_run], thread_id)
AppController-->>Client: return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @akitaSummer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces enhanced observability for streaming operations within the LangChain integration by adding a tracing mechanism to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds trace logging capabilities to the stream method of CompiledStateGraphObject, similar to what already exists for the invoke method. The changes include wrapping the stream method to inject a tracer and adding a new test case to verify this functionality.
My review found a critical issue in the implementation of the stream wrapper that would break the streaming functionality. There is also a related bug in the new test controller. Additionally, I've suggested some minor improvements for performance and code style in the test code. Please see the detailed comments for suggestions on how to fix these issues.
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
Outdated
Show resolved
Hide resolved
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
Outdated
Show resolved
Hide resolved
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @plugin/langchain/lib/graph/CompiledStateGraphObject.ts:
- Around line 51-56: The stream wrapper currently awaits originalStream and
casts its return to any; change the signature of graph.stream to explicitly
return the async-iterable stream type (IterableReadableStream or the project's
equivalent) and remove the await and the "as any" cast, i.e. have
graph.stream(...) return originalStream.call(graph, input, config) typed as
IterableReadableStream; keep the tracer insertion logic but ensure the function
return type matches the async-iterable so TypeScript enforces correct usage.
🧹 Nitpick comments (1)
plugin/langchain/test/llm.test.ts (1)
81-88: Test covers the new streaming endpoint but log file assertion may be flaky.The test correctly verifies the
/llm/graph-streamendpoint response. However, the log file assertion (lines 86-87) has potential issues:
- Race condition: Log content might not be flushed to disk immediately after the HTTP request completes.
- False positives: If the log file persists between test runs, the assertion could pass even if the current request didn't log correctly.
Consider adding a small delay before reading the log, or clearing/truncating the log file before the test to ensure the assertion is reliable.
♻️ Optional: Add log cleanup before test
it('should graph stream work', async () => { + const traceLogPath = path.join(__dirname, 'fixtures/apps/langchain/logs/egg-app/egg-app-web.log'); + // Clear log file before test to avoid false positives + await fs.writeFile(traceLogPath, '').catch(() => {}); + await app.httpRequest() .get('/llm/graph-stream') .expect(200, { value: 'hello graph toolhello world' }); - const traceLogPath = path.join(__dirname, 'fixtures/apps/langchain/logs/egg-app/egg-app-web.log'); const logContent = await fs.readFile(traceLogPath, 'utf-8'); assert(logContent.includes('/llm/graph-stream] [agent_run][fooGraph]')); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugin/langchain/lib/graph/CompiledStateGraphObject.tsplugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.tsplugin/langchain/test/llm.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
plugin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Framework internal code in plugin packages should import types from '@eggjs/tegg-types' and decorators from '@eggjs/core-decorator'
Files:
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.tsplugin/langchain/test/llm.test.tsplugin/langchain/lib/graph/CompiledStateGraphObject.ts
**/test/fixtures/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store test fixtures in 'test/fixtures/' directories within packages
Files:
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
**/test/**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should be located in 'test/' directories within each package and use Mocha
Files:
plugin/langchain/test/llm.test.ts
plugin/**/*.test.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Files:
plugin/langchain/test/llm.test.ts
plugin/*/lib/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Plugin services and lifecycle hooks should be implemented in files within the 'lib/' directory
Files:
plugin/langchain/lib/graph/CompiledStateGraphObject.ts
🧠 Learnings (3)
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Applies to plugin/**/*.test.{ts,js} : Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Create subpath exports in 'eggjs/tegg' for optional features (aop, orm, dal, schedule, transaction, ajv, helper, standalone)
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Use 'eggjs/tegg/helper' for internal APIs like ModuleConfigUtil, LoaderFactory, and EggObjectLifeCycleContext in advanced usage scenarios
Applied to files:
plugin/langchain/test/llm.test.ts
🧬 Code graph analysis (1)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (1)
core/controller-decorator/src/decorator/http/HTTPMethod.ts (1)
HTTPMethod(7-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
🔇 Additional comments (3)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (2)
64-89: New streaming endpoint implementation looks correct.The
getStreammethod properly exercises the stream functionality with trace-log enabled. Since this is test fixture code, the implementation is acceptable.Minor observation: The filter/reduce logic (lines 85-87) duplicates the pattern in
get()(lines 58-60). For test fixtures, this is fine, but if you prefer DRY, a helper function could be extracted.
54-54: Trace-log tag addition is consistent with the stream endpoint.Enabling tracer logging for the existing
get()endpoint aligns with the new streaming functionality.plugin/langchain/test/llm.test.ts (1)
3-3: Import is appropriate for async file operations.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/langchain/test/llm.test.ts (1)
22-25: Duplicateafterhooks callingapp.close()twice.There are two
afterhooks that both callapp.close():
- Lines 22-25:
await app.close()andawait stopSSEServer()- Lines 43-45:
return app.close()Calling
app.close()twice could cause unexpected behavior or errors. Consider consolidating these into a singleafterhook.🔧 Proposed fix to consolidate after hooks
after(async () => { await app.close(); await stopSSEServer(); }); afterEach(() => { mm.restore(); }); before(async () => { mm(process.env, 'EGG_TYPESCRIPT', true); mm(process, 'cwd', () => { return path.join(__dirname, '..'); }); app = mm.app({ baseDir: path.join(__dirname, 'fixtures/apps/langchain'), framework: path.dirname(require.resolve('egg')), }); await app.ready(); }); - after(() => { - return app.close(); - });Also applies to: 43-45
🧹 Nitpick comments (1)
plugin/langchain/test/llm.test.ts (1)
75-82: Consider improving test robustness for log file assertions.The log file assertion has a few potential reliability concerns:
- Stale log data: The log file may contain traces from previous test runs, causing false positives.
- Missing error context: If the assertion fails, the error message won't indicate what was actually in the log.
Consider these improvements for more robust testing:
💡 Optional: More robust log assertion
it('should graph work', async () => { + const traceLogPath = path.join(__dirname, 'fixtures/apps/langchain/logs/egg-app/egg-app-web.log'); + // Get initial log size to check only new content + let initialSize = 0; + try { + const stats = await fs.stat(traceLogPath); + initialSize = stats.size; + } catch { + // File may not exist yet + } + await app.httpRequest() .get('/llm/graph') .expect(200, { value: 'hello graph toolhello world' }); - const traceLogPath = path.join(__dirname, 'fixtures/apps/langchain/logs/egg-app/egg-app-web.log'); - const logContent = await fs.readFile(traceLogPath, 'utf-8'); - assert(logContent.includes('/llm/graph] [agent_run][fooGraph]')); + + const fullContent = await fs.readFile(traceLogPath, 'utf-8'); + const newContent = fullContent.slice(initialSize); + assert(newContent.includes('/llm/graph] [agent_run][fooGraph]'), + `Expected trace log to contain '/llm/graph] [agent_run][fooGraph]', but got: ${newContent.slice(0, 500)}`); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.tsplugin/langchain/test/llm.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
plugin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Framework internal code in plugin packages should import types from '@eggjs/tegg-types' and decorators from '@eggjs/core-decorator'
Files:
plugin/langchain/test/llm.test.tsplugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
**/test/**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should be located in 'test/' directories within each package and use Mocha
Files:
plugin/langchain/test/llm.test.ts
plugin/**/*.test.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Files:
plugin/langchain/test/llm.test.ts
**/test/fixtures/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store test fixtures in 'test/fixtures/' directories within packages
Files:
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts
🧠 Learnings (3)
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Applies to plugin/**/*.test.{ts,js} : Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Create subpath exports in 'eggjs/tegg' for optional features (aop, orm, dal, schedule, transaction, ajv, helper, standalone)
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Use 'eggjs/tegg/helper' for internal APIs like ModuleConfigUtil, LoaderFactory, and EggObjectLifeCycleContext in advanced usage scenarios
Applied to files:
plugin/langchain/test/llm.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (1)
45-62: LGTM!The addition of
tags: [ 'trace-log' ]correctly enables trace logging for the graph invocation, which aligns with the PR objective of adding stream tracelog functionality. The test fixture properly exercises the new tracing capability.plugin/langchain/test/llm.test.ts (1)
1-4: LGTM!The
fs/promisesimport is the correct modern approach for async file operations in Node.js.
akitaSummer
force-pushed
the
fix/graph_stream
branch
from
a317986 to
c1b6168
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @plugin/langchain/test/llm.test.ts:
- Line 81: Remove the debug console.log call that prints logContent from the
test in plugin/langchain/test/llm.test.ts; locate the statement
"console.log('logContent: ', logContent);" (inside the relevant test case that
references logContent) and delete it so tests do not emit extraneous debug
output.
- Around line 79-82: The test is flaky because it reads the log file directly
(traceLogPath) without ensuring a clean file state or waiting for the log to be
flushed; update the test to ensure reproducible reads by (1) cleaning/truncating
the log file in a beforeEach (remove or write empty file at traceLogPath) to
avoid stale entries, (2) replacing the single fs.readFile call with a short
retry/wait loop (read, if ENOENT or missing marker then sleep a few ms and retry
up to a timeout) before asserting logContent includes '[agent_run][fooGraph]',
and (3) fail fast with a clear message if file remains missing after retries;
alternatively, if available, use egg-mock or programmatic log capture instead of
reading traceLogPath directly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/langchain/test/llm.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
plugin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Framework internal code in plugin packages should import types from '@eggjs/tegg-types' and decorators from '@eggjs/core-decorator'
Files:
plugin/langchain/test/llm.test.ts
**/test/**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should be located in 'test/' directories within each package and use Mocha
Files:
plugin/langchain/test/llm.test.ts
plugin/**/*.test.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Files:
plugin/langchain/test/llm.test.ts
🧠 Learnings (3)
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Applies to plugin/**/*.test.{ts,js} : Use 'egg-mock' with 'app.mockModuleContextScope()' for testing plugin modules in isolation
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Create subpath exports in 'eggjs/tegg' for optional features (aop, orm, dal, schedule, transaction, ajv, helper, standalone)
Applied to files:
plugin/langchain/test/llm.test.ts
📚 Learning: 2026-01-10T12:35:11.421Z
Learnt from: CR
Repo: eggjs/tegg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T12:35:11.421Z
Learning: Use 'eggjs/tegg/helper' for internal APIs like ModuleConfigUtil, LoaderFactory, and EggObjectLifeCycleContext in advanced usage scenarios
Applied to files:
plugin/langchain/test/llm.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Analyze (typescript)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (20)
- GitHub Check: Analyze (javascript)
- GitHub Check: Runner-ubuntu (16)
akitaSummer
force-pushed
the
fix/graph_stream
branch
2 times, most recently
from
b81ddfd to
002c9a8
Compare
akitaSummer
changed the title
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
New Features
Logging
Tests
✏️ Tip: You can customize this high-level summary in your review settings.